-
-
Notifications
You must be signed in to change notification settings - Fork 450
Improve admin theme handling #4756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Install script shouldn't be needed, because the standard admin theme does fallback to the new base theme. So no changes needed |
|
XML layout has changed. When i switch to this branch admin login has no bg and i see old legacy theme. Until layout cache is flushed. |
|
GitHub says the branch has conflicts that must be resolved. I am not able to use the online editor for checking. Here is the file name |
|
Just needs an rebase |
b3d605b to
d9302bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR improves admin theme handling by restructuring the theme configuration to use a package-based approach instead of theme-based switching. The changes move the admin theme from "default/default" and "default/openmage" to a package-level selection between "default" and "openmage" packages, both using the "default" theme.
Changes:
- Restructures admin theme configuration to use package selection instead of theme selection
- Adds fallback logic to handle non-existent themes and parent theme layout updates
- Updates file paths in configuration files to reflect the new "base/default" structure
Reviewed changes
Copilot reviewed 7 out of 776 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| app/design/adminhtml/base/default/etc/theme.xml | Adds new theme configuration file for base/default admin theme |
| app/code/core/Mage/Core/Model/Layout/Update.php | Implements layout update fallback for parent themes |
| app/code/core/Mage/Core/Model/Design/Fallback.php | Adds theme existence check and fallback to default theme |
| app/code/core/Mage/Adminhtml/etc/config.xml | Moves openmage configuration from theme to package level |
| app/code/core/Mage/Adminhtml/Controller/Action.php | Updates theme selection logic to use package instead of theme |
| .phpstan.dist.neon | Updates file paths from default/default to base/default |
| .phpstan.dist.baseline.neon | Updates file paths from default/default to base/default |
Signed-off-by: Frank Rochlitzer <[email protected]>
Add @param Mage_Core_Model_Config_Element $updates and @return Mage_Core_Model_Config_Element to improve API documentation. Co-authored-by: Copilot <[email protected]>
Explanation for array manipulation Co-authored-by: Copilot <[email protected]>
Removed unnecessary docblock comments from the method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 778 changed files in this pull request and generated 5 comments.
Co-authored-by: Copilot <[email protected]>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 778 changed files in this pull request and generated 2 comments.
| $layoutXml = null; | ||
| $elementClass = $this->getElementClass(); | ||
| $updatesRoot = Mage::app()->getConfig()->getNode($area . '/layout/updates'); | ||
| $updatesRoot = $this->addFallbackThemesLayoutUpdates($updatesRoot); |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new method addFallbackThemesLayoutUpdates() is called here but there is no test coverage for this new behavior. Consider adding tests to verify that layout updates from parent themes are correctly merged into the fallback chain.




Description (*)
Done by Suggestion of #1216 (comment)
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)